refactor(computer-use): phase-0 quick wins from computer-use/browser-control audit - #1777
Merged
Merged
Conversation
added 3 commits
July 26, 2026 17:44
- Enforce the browser/desktop boundary guard in ComputerUse dispatch (previously unreachable dead code); narrow browser detection to Chromium-family so Firefox/Safari users are not locked out - Remove ghost tool references (ComputerUseMouseStep/MousePrecise/ MouseClick) from model-visible prompts, errors and MiniApp deny list - Validate CDP /json/version before labelling a headless session; stop hinting users to expose a debug port on their everyday browser - Trim visual-only actions from the text-only ComputerUse schema and reject them at runtime with a clear error - Gate debug screenshot persistence behind an env flag with retention - Guard scroll_x/scroll_y with the display-bounds check like other pointer actions - Delete dead compat shims (computer_use_input/result/verification, AIFeaturesConfig) and their boundary-check rules - Unify ai.computer_use_enabled subscription into a shared hook with a consistent initial value; close the slash-command bypass of the ComputerUse mode gate; correct over-promising toggle copy - Align browser routing guidance across mode prompts and the agent-browser skill (ControlHub-first ladder)
bobleer
pushed a commit
to bobleer/BitFun
that referenced
this pull request
Jul 26, 2026
…on safe An end-to-end review of the consolidated browser path (walking a real task: open -> log in -> find -> click -> observe -> read) found the advertised workflow could not actually be executed by a model. Results the model never saw: - The tool pipeline prefers `result_for_assistant`, and every browser action passed only a one-line count there, so `snapshot` returned literally "Snapshot: 42 interactive elements" and the @en refs the documented connect -> snapshot -> click flow depends on never reached the model. Same for list_pages/tab_query (no page ids, so switch_page was unusable), get_html, cookies, network/console/errors and meta.capabilities. Each now returns the payload the next decision needs; screenshot returns a real image attachment instead of a byte count and no longer duplicates base64 in the data field. Actions that silently did the wrong thing: - snapshot renumbered @en from 1 without clearing the previous pass's data-cdp-ref attributes, so after scrolling (viewport-filtered numbering) a stale attribute earlier in document order won the querySelector and click acted on the wrong element while reporting success. Refs are now cleared across the document, open shadow roots and same-origin frames before renumbering. - click/hover dispatched mouse events at a point without hit-testing it; an occluded target reported success. The center point is now verified with elementFromPoint and a covered target is refused with a recoverable error. - press_key sent no text/windowsVirtualKeyCode, so Enter never triggered native form submission. - browser.fetch wrapped CORS/transport failures in a "Fetched <url>" success line; they are now structured errors that explain the same-origin constraint, and HTTP status is surfaced in the summary while the response body is preserved. Frontmost-app detection (regression from the guard activated in GCWing#1777): - On Windows the foreground "name" is the window *title*, and the Chromium check matched bare substrings, so any window titled "Search" matched "arc" and had all desktop input blocked. Foreground apps now carry a process_name (executable basename on Windows, process name on macOS) and matching is whole-identity, falling back to token matching on the display name only when no identity is available. - The guard covers the app_*/interactive_*/visual_* variants that could previously bypass it, exempts the alt+tab/command+tab focus switch so a non-browser task can escape, and short-circuits on explicit non-browser selectors instead of querying the host. Also: select/wait accept @en refs, stale refs report STALE_REF instead of a generic not-found, unresolvable cross-origin iframes are explained instead of retried forever, prompts no longer promise the user's login state (connect uses BitFun's managed profile), open_builtin is routed as the display-only path, prompts stop pointing unconditionally at the default-off ComputerUse tool, and the agent-browser skill no longer argues against itself when explicitly invoked.
bobleer
added a commit
that referenced
this pull request
Jul 26, 2026
…th (#1779) * refactor(browser-control): consolidate to a single default browser path The audit found seven overlapping browser-control paths; the model oscillating between two full automation stacks (agent-browser CLI vs ControlHub CDP, each with its own @en ref namespace, browser instance and login state) was the largest source of failed browser operations. - Make agent-browser default-off in every mode so the ControlHub browser domain is the single default path; explicit /skill invocation and user overrides still opt in (extend the default-hidden whitelist so explicit invocation keeps working) - Fix iframe click coordinates: element_center now accumulates ancestor frame offsets along the frameElement chain; cross-origin frames surface a structured error advising snapshot + re-target; missing coords error instead of clicking (0,0) - Remove the dead frame/frame_main actions and the unread active_frame session state they wrote; they never affected execution context - Emit structured error codes with recovery instructions at the source (element not found -> NOT_FOUND "take a new snapshot", dead session -> WRONG_TAB "reconnect or switch_page", timeouts -> TIMEOUT) instead of relying on phrase-matching classification; phrase matching stays as fallback only - State non-observability where control leaves the CDP stack: open_builtin marks the panel display-only, open_url/open_file explain the opened window cannot be observed or controlled and route to the right tool, WebFetch description routes login/JS pages to the browser domain - Record the product decision that ai.computer_use_enabled and browser control are independent switches (browser control stays available when computer use is off) in the refactor plan * fix(browser-control): make browser results actionable and app detection safe An end-to-end review of the consolidated browser path (walking a real task: open -> log in -> find -> click -> observe -> read) found the advertised workflow could not actually be executed by a model. Results the model never saw: - The tool pipeline prefers `result_for_assistant`, and every browser action passed only a one-line count there, so `snapshot` returned literally "Snapshot: 42 interactive elements" and the @en refs the documented connect -> snapshot -> click flow depends on never reached the model. Same for list_pages/tab_query (no page ids, so switch_page was unusable), get_html, cookies, network/console/errors and meta.capabilities. Each now returns the payload the next decision needs; screenshot returns a real image attachment instead of a byte count and no longer duplicates base64 in the data field. Actions that silently did the wrong thing: - snapshot renumbered @en from 1 without clearing the previous pass's data-cdp-ref attributes, so after scrolling (viewport-filtered numbering) a stale attribute earlier in document order won the querySelector and click acted on the wrong element while reporting success. Refs are now cleared across the document, open shadow roots and same-origin frames before renumbering. - click/hover dispatched mouse events at a point without hit-testing it; an occluded target reported success. The center point is now verified with elementFromPoint and a covered target is refused with a recoverable error. - press_key sent no text/windowsVirtualKeyCode, so Enter never triggered native form submission. - browser.fetch wrapped CORS/transport failures in a "Fetched <url>" success line; they are now structured errors that explain the same-origin constraint, and HTTP status is surfaced in the summary while the response body is preserved. Frontmost-app detection (regression from the guard activated in #1777): - On Windows the foreground "name" is the window *title*, and the Chromium check matched bare substrings, so any window titled "Search" matched "arc" and had all desktop input blocked. Foreground apps now carry a process_name (executable basename on Windows, process name on macOS) and matching is whole-identity, falling back to token matching on the display name only when no identity is available. - The guard covers the app_*/interactive_*/visual_* variants that could previously bypass it, exempts the alt+tab/command+tab focus switch so a non-browser task can escape, and short-circuits on explicit non-browser selectors instead of querying the host. Also: select/wait accept @en refs, stale refs report STALE_REF instead of a generic not-found, unresolvable cross-origin iframes are explained instead of retried forever, prompts no longer promise the user's login state (connect uses BitFun's managed profile), open_builtin is routed as the display-only path, prompts stop pointing unconditionally at the default-off ComputerUse tool, and the agent-browser skill no longer argues against itself when explicitly invoked. * fix(browser-control): pierce shadow roots in the click hit-test The occlusion hit-test compared document.elementFromPoint against the target with contains(), but elementFromPoint stops at a shadow host and host.contains(shadowChild) is false — so every element inside an open shadow root (which resolve/snapshot deliberately support) was reported as occluded by its own host and refused. Descend through open shadow roots' elementFromPoint before comparing. --------- Co-authored-by: bowen628 <bowen628@noreply.gitcode.com>
1688mengdie
pushed a commit
to 1688mengdie/BitFun
that referenced
this pull request
Jul 26, 2026
…th (GCWing#1779) * refactor(browser-control): consolidate to a single default browser path The audit found seven overlapping browser-control paths; the model oscillating between two full automation stacks (agent-browser CLI vs ControlHub CDP, each with its own @en ref namespace, browser instance and login state) was the largest source of failed browser operations. - Make agent-browser default-off in every mode so the ControlHub browser domain is the single default path; explicit /skill invocation and user overrides still opt in (extend the default-hidden whitelist so explicit invocation keeps working) - Fix iframe click coordinates: element_center now accumulates ancestor frame offsets along the frameElement chain; cross-origin frames surface a structured error advising snapshot + re-target; missing coords error instead of clicking (0,0) - Remove the dead frame/frame_main actions and the unread active_frame session state they wrote; they never affected execution context - Emit structured error codes with recovery instructions at the source (element not found -> NOT_FOUND "take a new snapshot", dead session -> WRONG_TAB "reconnect or switch_page", timeouts -> TIMEOUT) instead of relying on phrase-matching classification; phrase matching stays as fallback only - State non-observability where control leaves the CDP stack: open_builtin marks the panel display-only, open_url/open_file explain the opened window cannot be observed or controlled and route to the right tool, WebFetch description routes login/JS pages to the browser domain - Record the product decision that ai.computer_use_enabled and browser control are independent switches (browser control stays available when computer use is off) in the refactor plan * fix(browser-control): make browser results actionable and app detection safe An end-to-end review of the consolidated browser path (walking a real task: open -> log in -> find -> click -> observe -> read) found the advertised workflow could not actually be executed by a model. Results the model never saw: - The tool pipeline prefers `result_for_assistant`, and every browser action passed only a one-line count there, so `snapshot` returned literally "Snapshot: 42 interactive elements" and the @en refs the documented connect -> snapshot -> click flow depends on never reached the model. Same for list_pages/tab_query (no page ids, so switch_page was unusable), get_html, cookies, network/console/errors and meta.capabilities. Each now returns the payload the next decision needs; screenshot returns a real image attachment instead of a byte count and no longer duplicates base64 in the data field. Actions that silently did the wrong thing: - snapshot renumbered @en from 1 without clearing the previous pass's data-cdp-ref attributes, so after scrolling (viewport-filtered numbering) a stale attribute earlier in document order won the querySelector and click acted on the wrong element while reporting success. Refs are now cleared across the document, open shadow roots and same-origin frames before renumbering. - click/hover dispatched mouse events at a point without hit-testing it; an occluded target reported success. The center point is now verified with elementFromPoint and a covered target is refused with a recoverable error. - press_key sent no text/windowsVirtualKeyCode, so Enter never triggered native form submission. - browser.fetch wrapped CORS/transport failures in a "Fetched <url>" success line; they are now structured errors that explain the same-origin constraint, and HTTP status is surfaced in the summary while the response body is preserved. Frontmost-app detection (regression from the guard activated in GCWing#1777): - On Windows the foreground "name" is the window *title*, and the Chromium check matched bare substrings, so any window titled "Search" matched "arc" and had all desktop input blocked. Foreground apps now carry a process_name (executable basename on Windows, process name on macOS) and matching is whole-identity, falling back to token matching on the display name only when no identity is available. - The guard covers the app_*/interactive_*/visual_* variants that could previously bypass it, exempts the alt+tab/command+tab focus switch so a non-browser task can escape, and short-circuits on explicit non-browser selectors instead of querying the host. Also: select/wait accept @en refs, stale refs report STALE_REF instead of a generic not-found, unresolvable cross-origin iframes are explained instead of retried forever, prompts no longer promise the user's login state (connect uses BitFun's managed profile), open_builtin is routed as the display-only path, prompts stop pointing unconditionally at the default-off ComputerUse tool, and the agent-browser skill no longer argues against itself when explicitly invoked. * fix(browser-control): pierce shadow roots in the click hit-test The occlusion hit-test compared document.elementFromPoint against the target with contains(), but elementFromPoint stops at a shadow host and host.contains(shadowChild) is false — so every element inside an open shadow root (which resolve/snapshot deliberately support) was reported as occluded by its own host and refused. Descend through open shadow roots' elementFromPoint before comparing. --------- Co-authored-by: bowen628 <bowen628@noreply.gitcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
A full audit of the computer-use / browser-control stack (internal code review plus benchmarking against trycua/cua, OpenAI Codex CLI, Anthropic computer-use-demo, browser-use, and playwright-mcp) surfaced 33 issues (6 critical): 7 overlapping browser-control paths, a single god-tool with 40 actions / 7 click dialects / 4 coordinate systems, and safety boundaries that were never enforced. The full diagnosis and a six-phase refactor roadmap are added in
docs/plans/computer-use-refactor-plan.md.This PR lands the phase-0 quick wins: correctness and safety fixes only, no architectural changes.
Changes
Safety / correctness
desktop_action_targets_browserwas only wired to an unreachable fallthrough inhandle_desktop; it now runs atComputerUsedispatch, rejecting physical input actions while a Chromium-family browser is frontmost and pointing the model at the ControlHub browser domain. Firefox/Safari are removed from the browser detection table — CDP does not support them, so blocking desktop input there locked users out in both directions.browser.connect { mode: "headless" }now verifies via the CDP/json/versionhandshake that the endpoint really is a headless browser; previously it could attach to the user's real logged-in browser and label the session "Headless test browser".--remote-debugging-porton their everyday browser; they point at the BitFun-managed profile instead.BITFUN_COMPUTER_USE_DEBUG_SCREENSHOTS=1) and keeps only the 20 most recent files; previously every screenshot was unconditionally written into the user's workspace.scroll_x/scroll_ynow go through the same display-bounds check as other pointer actions.Model execution quality
ComputerUseMouseStep/MousePrecise/MouseClick): several model-visible prompts and error messages still told the model to call these long-deleted tools, causing failure loops.build_interactive_view/interactive_click/build_visual_mark_view/visual_click) from the text-only ComputerUse schema and reject them at runtime with a clear error.@eNref namespaces.Cleanup
computer_use_input.rs,computer_use_result.rs,computer_use_verification.rs(plus its core-boundaries rule) andAIFeaturesConfig.tsx.ai.computer_use_enabledsubscriptions into a shareduseComputerUseEnabled()hook with a consistent initial value offalse(they previously disagreed, causing a misleading first render).Verification
cargo check --workspacepassescargo test -p bitfun-core computer_use: 22 passed (includes new guard-reachability and text-only schema drift tests)cargo test -p bitfun-core control_hub: 29 passed (includes 3 new headless validation tests)cargo test -p bitfun-agent-tools: 103 passednode scripts/core-boundaries/checker.mjspassestsc --noEmitandeslintpassFollow-up
Phases 1–6 from
docs/plans/computer-use-refactor-plan.md(remaining dead-code cleanup, 40→~15 action-space consolidation, per-session host state, provider-native computer-use tool mapping, single browser stack, config/permission/peer deny-table single source of truth) will follow as separate PRs.